Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(Landscape): Send hostagent uid to WSL landscape client #360

Merged
merged 18 commits into from
Oct 25, 2023

Conversation

EduardGomezEscandell
Copy link
Contributor

The landscape client needs field hostagent_uid to match up the WSL distro with the Windows hostagent. See canonical/landscape-client#189

Now the config needs to know the hostagent UID in order to put it in the provisioning tasks. To avoid circular dependencies, we shift the ownership of this data from the Landscape proservice to the Config.
At the moment it is put in the registry, but this is something that should not be permanent (what if the user has no write permission?).

From the user perspective, this means they don't even need to know about this hostagent UID.


UDENG-1588

@EduardGomezEscandell EduardGomezEscandell self-assigned this Oct 24, 2023
@EduardGomezEscandell EduardGomezEscandell changed the title Send hostagent uid to WSL landscape client feat(Landscape): Send hostagent uid to WSL landscape client Oct 24, 2023
@EduardGomezEscandell EduardGomezEscandell force-pushed the landscape-client-hostagent-uid branch from 92103a1 to 9cbf554 Compare October 24, 2023 11:35
@EduardGomezEscandell EduardGomezEscandell marked this pull request as ready for review October 24, 2023 15:08
Copy link
Collaborator

@didrocks didrocks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small comments and suggestions ahead!

windows-agent/internal/config/config.go Show resolved Hide resolved
windows-agent/internal/config/config_test.go Outdated Show resolved Hide resolved
windows-agent/internal/config/config_test.go Outdated Show resolved Hide resolved
windows-agent/internal/config/config_test.go Outdated Show resolved Hide resolved
windows-agent/internal/config/config_test.go Outdated Show resolved Hide resolved
It was reading the registry twice, with two mutex lock-unlocks. If
we now add the need to read the hostagent UID, that would be a third
time. So we simplify it to more basic operations.
This responsability has been shifted to the config.
There are now fewer, but different failure modes.
Otherwise, the Landcape back-end cannot match the WSL distros to this
any agent.
Done in anticiption of adding a third test for the LandscapeAgentUID
getter.
All default values are empty strings, and the linter was also
complaining about readValue always receiving the same value for its
'defaultValue' argument.

Hence, I simplified the code so that there are no default values, or
rather the default values are always "". We can bring them back if we
ever need one of the defaults to be non-empty.
@EduardGomezEscandell EduardGomezEscandell force-pushed the landscape-client-hostagent-uid branch from 9cbf554 to c8b3916 Compare October 25, 2023 13:57
@EduardGomezEscandell
Copy link
Contributor Author

Force-pushed to resolve conflict. Real changes start under this comment.

Copy link
Collaborator

@didrocks didrocks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent! (once the tests pass ofc :))

@EduardGomezEscandell EduardGomezEscandell merged commit b3e7db9 into main Oct 25, 2023
28 checks passed
@EduardGomezEscandell EduardGomezEscandell deleted the landscape-client-hostagent-uid branch October 25, 2023 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants